Remove the trailing blank lines at the end of the file. This follows standard code style guidelines and maintains consistency across the codebase.

Remove the trailing blank lines at the end of the file. This follows standard code style guidelines and maintains consistency across the codebase.

Remove the trailing blank lines at the end of the file. This follows standard code style guidelines and maintains consistency across the codebase.

Remove the trailing blank line at the end of the file. This follows standard code style guidelines and maintains consistency across the codebase.

Remove the trailing blank line at the end of the file. This follows standard code style guidelines and maintains consistency across the codebase.

These commented-out lines should either be removed or uncommented if the checks are needed. Leaving commented-out code in production reduces maintainability. If these checks are intentionally disabled for this WIP PR, consider adding a TODO comment explaining why and when they should be re-enabled.

This 'lambda' is just a simple wrapper around a callable object. Use that object directly.

This 'lambda' is just a simple wrapper around a callable object. Use that object directly.

The class 'ProtobufSimpleMessageRowGen' does not override 'eq', but adds the new attribute _fields.
The class 'ProtobufSimpleMessageRowGen' does not override 'eq', but adds the new attribute _binary_col_name.

'except' clause does nothing but pass and there is no explanatory comment.

The second return value from sparkTypeToCudfId (the encoding) is being ignored and replaced with the encoding determined from the protobuf type. This suggests that sparkTypeToCudfId should not return an encoding at all, or its second return value should be used. Consider refactoring sparkTypeToCudfId to only return the type ID, since the encoding is determined elsewhere based on the protobuf field type.

The sparkTypeToCudfId function always returns ENC_DEFAULT as the second tuple element, regardless of the input type. However, this encoding value is not actually used by the caller in ProtobufExprShims.scala (line 199), which discards it with an underscore. Consider simplifying this function to return only the type ID as an Int, since the encoding is determined separately based on the protobuf wire type.

This utility class appears to be unused in the current implementation. The ProtobufExprShims.scala uses Spark's ProtobufUtils via reflection (buildMessageDescriptorWithSparkProtobuf) instead of this custom utility. Consider removing this file if it's not needed for future work, or add documentation explaining its intended purpose if it's meant for upcoming features.

The classpath separator is hardcoded to colon (:) which is Unix-specific. On Windows, the classpath separator should be semicolon (;). Consider using os.pathsep instead of ':' to make this code platform-independent.

The comment refers to "Patch 1" which may be unclear to readers who are not familiar with the development context. Consider rephrasing to something like "initial implementation" or "this PR" for better clarity.

The temporary descriptor file is marked for deletion on JVM exit using deleteOnExit(), but there's no guarantee this will be called, especially in cases of abrupt termination or long-running processes. Consider using a try-finally block or resource management approach to ensure cleanup, or document that these temp files are expected to persist in some scenarios.

This 'lambda' is just a simple wrapper around a callable object. Use that object directly.

If an exception occurs during the loop (lines 85-95), the columns that were already created in fullChildren will be leaked because the finally block only closes non-null entries, but an exception could leave fullChildren partially populated. The finally block should be inside the loop body or use a more robust pattern.

Additionally, if cudf.ColumnVector.makeStruct on line 96 throws an exception, the columns in fullChildren will be leaked because they've already been closed in the finally block, but makeStruct hasn't taken ownership yet.

The safer pattern would be:

try {
  for (i <- fullSchema.fields.indices) {
    val child = try {
      if (decodedIdx < decodedFieldIndices.length && decodedFieldIndices(decodedIdx) == i) {
        val col = decodedStruct.getChildColumnView(decodedIdx).copyToColumnVector()
        decodedIdx += 1
        col
      } else {
        GpuFromProtobuf.createNullColumn(fullSchema.fields(i).dataType, numRows)
      }
    } catch {
      case e: Throwable =>
        fullChildren.take(i).foreach(c => if (c != null) c.close())
        throw e
    }
    fullChildren(i) = child
  }
  cudf.ColumnVector.makeStruct(numRows, fullChildren: _*)
} finally {
  fullChildren.foreach(c => if (c != null) c.close())
}

Or use withResource pattern more carefully to ensure proper cleanup ordering.

The StructType comparison doesn't account for nullable flags on struct fields, which could lead to false positive matches. Two structs with the same field names and types but different nullability should not be considered equal for this matching purpose.

Consider: